Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rofiles-fuse: Support breaking hardlinks to fs-verity files #3174

Closed

Conversation

alexlarsson
Copy link
Member

In case fs-verity is in used for the repo objects, and something like "rpm-ostree apply-live" uses rofiles-fuse with --copyup, then writing to a hard-linked file fails to copy up, like this:

echo foo > /a/rofile-mnt/a-file
/a/rofile-mnt/a-file: Operation not permitted

The reason for this is that do_write() starts by opening the file non-truncating for writing, stat:ing it and then calling verify_write_or_copyup(). It is expecting the the open(write) to succeed, however, in the fs-verity case any open with write fails with EPERM.

We fix this by delaying the EPERM failure, only reporting it when the file descriptor needs to be used. In the case this triggered a copyup the file descriptor will be reopened, and in this case we will not get the EPERM anymore.

To simplify this code the fd variable now uses glnx_autofd.

This fixes coreos/rpm-ostree#4827

In case fs-verity is in used for the repo objects, and something like
"rpm-ostree apply-live" uses rofiles-fuse with --copyup, then writing
to a hard-linked file fails to copy up, like this:

echo foo > /a/rofile-mnt/a-file
/a/rofile-mnt/a-file: Operation not permitted

The reason for this is that do_write() starts by opening the file
non-truncating for writing, stat:ing it and then calling
verify_write_or_copyup(). It is expecting the the open(write) to
succeed, however, in the fs-verity case any open with write fails with
EPERM.

We fix this by delaying the EPERM failure, only reporting it when the
file descriptor needs to be used. In the case this triggered a copyup
the file descriptor will be reopened, and in this case we will not get
the EPERM anymore.

To simplify this code the fd variable now uses glnx_autofd.

This fixes coreos/rpm-ostree#4827
@cgwalters
Copy link
Member

Looks sane, would be good to have a new test case...I was looking at this but the existing test case is failing for me with

PASS: tests/test-rofiles-fuse.sh 10 flock
++ stat -c %i checkout-test2/firstfile
+ firstfile_orig_inode=98938420
+ echo -n truncating
/var/srv/walters/src/github/ostreedev/ostree/tests/test-rofiles-fuse.sh: line 157: mnt/firstfile: Input/output error
++ report_err
++ local exit_status=1
Unexpected nonzero exit status 1 while running: echo -n truncating > mnt/firstfile

which I haven't debugged yet.

Just thinking about this though...I bet we could test your change here even if the host environment doesn't use fsverity by simply using chmod 0 on a file - (and dropping CAP_DAC_OVERRIDE if we happen to have it, but the unit tests are intended to run as non-root anyways).

@alexlarsson
Copy link
Member Author

I'll have a look at adding a test tomorrow

@alexlarsson
Copy link
Member Author

Not sure chmod is enough, that would give a EACCESS instead of EPERM, no? Maybe some immutable attr would workj?

@cgwalters
Copy link
Member

One problem here is that we do copyup if the calling process just does e.g. chmod too, and we're not handling that here.

@cgwalters
Copy link
Member

Ended up trying out an alternative flow in #3175 - mind taking a look?

@alexlarsson
Copy link
Member Author

The other approach seems fine to me too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

drop rofiles-fuse, start using overlayfs
2 participants